Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: add defaultTo #357681

Merged
merged 1 commit into from
Dec 8, 2024
Merged

lib: add defaultTo #357681

merged 1 commit into from
Dec 8, 2024

Conversation

szlend
Copy link
Member

@szlend szlend commented Nov 20, 2024

I'm surprised this is not a thing yet. I swear I have a need for this function every single day.

If you have some long nullable expression like foo.bar.baz, or a function call foo 123, then using if a != null then a else b can get unnecessarily long, and even unnecessarily re-evaluated. So you usually need to wrap it into a let binding like let a = foo.bar.baz; if a != null then a else b. Of course this is annoying.

I propose adding a lib.trivial.defaultTo function to nixpkgs to handle this very common use case.

Example usage:

lib.defaultTo "default" null
=> "default"
lib.defaultTo "default" "foo"
=> "foo"
lib.defaultTo "default" false
=> false

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@szlend szlend requested a review from roberth November 20, 2024 22:00
@github-actions github-actions bot added the 6.topic: lib The Nixpkgs function library label Nov 20, 2024
@nix-owners nix-owners bot requested a review from infinisil November 20, 2024 22:01
@roberth
Copy link
Member

roberth commented Nov 20, 2024

As a Haskeller, this function confused me briefly, as its either :: (a -> r) -> (b -> r) -> Either a b -> r function is also typically called with two arguments, but does something very different.
While Haskell isn't Nix, Nix does have an attrset shape close to Rust's Err type, which is isomorphic to Either, which sees similar use. This would be the shape of the return value of tryEval.

A problem with the name "either" is that in natural language use, it does not convey a bias towards the one or the other. (And also the Haskell type suffers from this, coincidentally)

These two reasons lead me to believe that a better name could be found for this function.
I'd be ok with coalesce from SQL (only objection being that variable arity could be thought of as a list, which I would not advocate for, but marks a difference).
Other ideas would be

  • fallback (suffering from the same lack of "bias")
  • otherwise, requiring that you imagine the function name between the arguments, which is not too uncommon I think
    • orElse
  • orNullable (hinting at the "nullable" family of functions; this would be the 3rd member)

Definitiely a 👍 for adding a function that performs this pretty fundamental operation

@olivia-fl
Copy link
Contributor

orElse collides with the rust convention of fn or_else(&self: &C<T>, other: impl Fn() -> T) -> C<T>. The rust name for the null coalescing function is or, which is already taken in nixpkgs. unless is slightly shorter an option in the same vein as otherwise, with behavior kinda similar to the lisp macro unless.

@szlend
Copy link
Member Author

szlend commented Nov 20, 2024

Other ideas would be ...

I feel like these don't read all that well in regular function form, but would read great with the pipe operator:

null |> fallback "foo"
null |> otherwise "foo"
null |> orElse "foo"
null |> orNullable "foo"

vs.

fallback null "foo"
otherwise null "foo"
orElse null "foo"
orNullable null "foo"

Alas, we are not there yet... But maybe looking ahead?

As it stands, I feel like coalesce/fallback read the best imo.

Edit: Actually my assumption with the pipe operator might be wrong, need to re-check the RFC.

@llakala
Copy link
Contributor

llakala commented Nov 21, 2024

Of the names suggested so far, fallback is definitely my favorite. Some other ideas, that try to clarify bias:

  • prefer (with the idea of an implicit "over" between the arguments, resulting in the sentence "Prefer A over B."
  • try (matching the idea of a try/catch block)

I like prefer, and it makes me think of an alternate idea that may be clearer as a user. Rather than taking two values as arguments, it would take a list of values, and simply return the first value that wasn't null. The order of the values would represent the preference. I think this method of interaction makes it clearer which values will be preferred.

Don't want to bikeshed, though - just another idea that shouldn't prevent this PR from being merged.

@pluiedev
Copy link
Contributor

pluiedev commented Nov 21, 2024

I prefer otherwise: prefer sounds like a priority thing (cf. mkOverride or hiPrio/loPrio); try sounds too much like error handling, which it isn't; coalesce sounds way too complex for what it is and IMO may obfuscate its meaning, especially for non-native English speakers; unless might confuse people coming from Perl/Ruby as it's an if with an inverted condition there; and either confuses my Haskell brain for aforementioned reasons.

@szlend
Copy link
Member Author

szlend commented Nov 22, 2024

After sleeping on it, my favorite so far is definitely fallback:

lib.fallback foo.bar.baz "default"

I read it as: fallback foo.bar.baz on "default". I don't feel like it lacks bias.

@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented Nov 22, 2024

I agree this is a useful thing to have.
However: What about a scenario with multiple potentially null values? Might it make sense to also expose something like the following?

first_non_null = builtins.foldl' either null;

It'd operate on a list:

nix-repl> first_non_null [ null null 5 null "test" null 7]
5
nix-repl> first_non_null [ true null null 5 null "test" null 7]
true
nix-repl>

If we have either, this is trivial to build. But it'd feel right to have a "multi-fallback" too if we have a pairwise fallback.
Note this is not immediately equivalent to filtering for non-null and taking the first element. Taking the first element would fail to eval if no elements are non-null, while this would return null at the very end.

@szlend
Copy link
Member Author

szlend commented Nov 23, 2024

If we're talking about a list fallback variant, then I would definitely want it to be a separate function yeah. I feel like it should be as close to the original if then else as possible and not allocate extra thunks or loop without early exit.

It's worth considering a name that would be applicable to both variants in that case, though that certainly makes thing more difficult. Maybe fallbackChain/fallbackList/lib.lists.fallback?

I don't know... Personally I feel like the vast majority of the time I just want the basic fallback a b so I'm not really all that invested in the list variant.

@wolfgangwalther
Copy link
Contributor

After sleeping on it, my favorite so far is definitely fallback:

lib.fallback foo.bar.baz "default"

I read it as: fallback foo.bar.baz on "default". I don't feel like it lacks bias.

Does it make sense to reverse the arguments?

map (lib.fallback "default") [ "a" null "b" null ]

This could be made clearer in the name, too, like lib.fallbackTo or lib.defaultTo.

@szlend
Copy link
Member Author

szlend commented Nov 25, 2024

I kinda prefer the fallback happening from left to right, but you're right that usually functions in Nix are written with partial application in mind, in which case it makes sense for the fallback value to come first. lib.fallbackTo/lib.defaultTo are definitely more fitting names imo if we go with that variant.

I'm happy to change it to any of these if there's consensus.

@szlend szlend force-pushed the lib-either branch 2 times, most recently from 3c1a433 to 23d8372 Compare December 1, 2024 09:40
@szlend szlend changed the title lib: add either lib: add defaultTo Dec 1, 2024
@szlend
Copy link
Member Author

szlend commented Dec 1, 2024

Changed the implementation to defaultTo and swapped arguments. Let me know what you think.

lib/trivial.nix Outdated Show resolved Hide resolved
Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@infinisil infinisil merged commit 944f474 into NixOS:master Dec 8, 2024
31 checks passed
@roberth roberth added the backport release-24.11 Backport PR automatically label Dec 22, 2024
@nix-backports
Copy link

nix-backports bot commented Dec 22, 2024

Successfully created backport PR for release-24.11:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants